fix: better handling of boot order when running v.CloudInit() method #108
Conversation
Fixes running the v.CloudInit() method in the case that the boot devices parameter is unset. Also adds some functions for messing with boot devices in general. Mostly because I'm extremely undecided with how we should alter the boot order when doing the CloudInit() method, or if we should even alter it in the first place. I'm guessing the reason this method changes the boot order is to make sure that the added cloud-init ISO is not the first boot device? So I'm not sure if we should just remove it from the boot order entirely, or if we should set it to the last boot device.
| @@ -171,6 +183,59 @@ func (v *VirtualMachine) CloudInit(ctx context.Context, device, userdata, metada | |||
| return task.WaitFor(ctx, 2) | |||
| } | |||
|
|
|||
| // MakeBootString takes a list of boot devices and returns a string of the format, "order=firstDevice;secondDevice" | |||
| func MakeBootString(devices ...string) string { | |||
There was a problem hiding this comment.
probably no reason to export this, let's make it internal
|
|
||
| // ParseBootString takes a boot string of the format, "order=firstDevice;secondDevice", and returns a slice of strings | ||
| // representing the boot devices in order. | ||
| func ParseBootString(bootS string) []string { |
There was a problem hiding this comment.
probably no reason to export this, let's make it internal
| } | ||
|
|
||
| // AppendDeviceToBootString adds a device to the end of the boot string. | ||
| func AppendDeviceToBootString(bootS, device string) string { |
There was a problem hiding this comment.
probably no reason to export this, let's make it internal
|
|
||
| // RemoveDeviceFromBootString takes a boot string and returns a boot string minus a given device. | ||
| // If the given device is already not present in the boot order, this is basically a no-op. | ||
| func RemoveDeviceFromBootString(bootS, device string) string { |
There was a problem hiding this comment.
probably no reason to export this, let's make it internal
|
|
||
| // MoveDeviceToEndOfBootString moves a device to the end of the boot order. | ||
| // If the given device is not present in the boot order, this adds it at the end. | ||
| func MoveDeviceToEndOfBootString(bootS, device string) string { |
There was a problem hiding this comment.
probably no reason to export this, let's make it internal
|
@b- i can see you're trying to make some things more useful for people by exporting some common funcs to do bootstrings. If you want to give people something like this perhaps it's time to do a BootString type with all of these things as receiver funcs? i like where youre going with this overall but i think it could be better and if we're going to add this either as is i'd say pull everything internal so there is no new api or let's just go for it and make an official type with these ideas available to people implementing this package in their code |
That all sounds good — I agree. I just don't really know how to do it better or else I would have! 😅 |
Fixes running the v.CloudInit() method in the case that the boot devices parameter is unset. (Of course I wouldn't expect such a VM to boot, but I intended to set that option after feeding it a cloud-init ISO anyway, so I was surprised that it failed...)
Also adds some functions for messing with boot devices in general. Mostly because I'm extremely undecided with how we should alter the boot order when doing the CloudInit() method, or if we should even alter it in the first place.
I'm guessing the reason this method changes the boot order is to make sure that the added cloud-init ISO is not the first boot device? So I'm not sure if we should just remove it from the boot order entirely, or if we should set it to the last boot device.
I'm not marking this as a draft because it's working code that's certainly not going to cause issues merging in, but there's an unused function in here and in general I can be pretty verbose in my function naming...